UCF101 fails if root directory ends in "/" #1703
Conversation
|
Is 2.7 support going to be dropped (given how 2.7 will be discontinued), or should I fix my solution? |
|
I also had issues applying transforms when using the UCF101 dataset. I made some changes so that way this was not an issue. I also noticed that the changes I made could affect a few other datasets (kinetics and hmdb51), so I made changes in those files as well. |
| transformed_video = [] | ||
| for counter, image in enumerate(video): | ||
| image = self.transform(image) | ||
| transformed_video.append(image) | ||
| video = torch.stack(transformed_video) |
There was a problem hiding this comment.
Those are unrelated changes that shouldn't be handled this way.
Instead, we are working on video transforms that can be applied to a video clip right away (they are currently private functions because their API will change and the default transforms will natively support video clips, but the private functions are in https://github.com/pytorch/vision/blob/master/torchvision/transforms/_transforms_video.py)
|
Thanks @Jbwasse2 for your contribution and sorry for the long delay before getting a reply. As far as my understanding goes, this PR adresses two things at once:
Regarding the second point, there is a PR that is introducing a new way to decode video datasets so might become redundant. @pmeier probably has more details about this point. As for the first point, there is currently an effort to migrate existing datasets to a new datasets API (as described here) and thus it might be best to wait for the migration before fixing the issue. The migration can be done by someone else of course @Jbwasse2 and due credit for identifying the bug and proposing a fix will be given to you. Does that work for you @Jbwasse2? |
|
@yassineAlouini given that this PR is ~2.5 years old, did you check if the problem actually persists? Regarding the other changes, we should follow the advice in #1703 (comment) and not do this here. You are right that #5422 will make probably make it obsolete anyway. |
That's a good point, I will check against the latest main branch and report back. 👌 |
|
Hello, thank you for the follow up. Your solution works for me. |
|
As suggested @pmeier, I downloaded the dataset and tried a root path with an ending "/" and it did work (see screenshot below). Is that what you had in mind or do you have some additional tests I can try? Let me know since I have the dataset available now. 👌
|
|
Thanks for the follow-up @yassineAlouini. Since the issue seems to have been fixed in the mean time and other changes will be handled in #5422, I'm closing this PR. Thanks for the effort @Jbwasse2 and @yassineAlouini! |
Previously when the UCF101 dataset was trying to find indices, if the root directory given ended with "/" it would clip the first character of the string that it was looking for which would cause the dataset to fail.
For example, if the dataset was trying to check if "./ucf_data/videos/Bowling/v_Bowling_g06_c01.avi'" index should be added to the indices list, it would instead check if "owling/v_Bowling_g06_c01.avi' is in the set "selected_files".
Changing the 1: to a 0: would only allow root directories ending with "/" to pass. So instead I propose using the PathLib library when checking paths.